-
Notifications
You must be signed in to change notification settings - Fork 1
replace extra method with string parameter #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
In plenary today @waldemarhorwat suggested doing "discard" and "short", which I'm also happy with. |
I think the parameter should use a consistent part of speech, preferably verbs. Past participles "discarded"/"omitted" and "undersized"/"truncated" would also be fine. |
"Discard", noun, "one that is cast off or rejected". |
😛 That's not the form being used here and you know it. |
What about “discard” and “shorten”? |
That seems fine. It's pretty much synonymous with |
I would throw in |
"full" is confusing to me because you're getting fewer outputs in that case. I'd maybe accept "only-full"? But that's kind of verbose. |
To address part of speech concerns, "discard" and "output-short", maybe? |
I really don't like having two almost identical methods which differ only in their handling of an edge case. This PR removes
sliding
and adds a string parameter towindows
which controls the behavior in the case of an undersized iterator, with possible values"discard"
(the default) or"truncate"
(the previous behavior forsliding
). Feel free to bikeshed the values if you have strong opinions. I left out"throw"
but I guess it's also a valid thing to want (edit: #25).I've chosen to go with a string argument instead of a boolean so that we can later turn this into an options bag if we really feel the need. Boolean would also be fine but makes it harder to do that change (because we said we'd allow coercion to boolean).
Fixes #23 (by causing it not to be a problem).